Code Review

PR #6: feat: add native AOT support

Summary

This PR adds comprehensive Native AOT support to AWS.AgentCore, introducing source-generated JSON serialization and AOT-safe strongly-typed overloads for both non-streaming and streaming scenarios. The approach is well-designed: reflection-based overloads are preserved for non-AOT users and properly annotated with [RequiresUnreferencedCode]/[RequiresDynamicCode], while new overloads accepting JsonTypeInfo enable zero-warning AOT publishing. The source generator is updated to emit the appropriate overload based on the new JsonContext property. Two sample apps demonstrate both programming models. Overall this is a high-quality PR with thorough documentation, good test coverage, and a clean API design. A few issues warrant attention before merge.

Important Issues - Should Fix

1. Custom ping handler return value is silently discarded - behavioral regression

   • File: src/AWS.AgentCore.SourceGenerator/AgentCoreStartupGenerator.cs
   • The generated ping handler code calls agent.Ping() but discards the return value. The old generated code was return agent.Ping() which returned the object to the framework for serialization. The new ping handler signature Func<IServiceProvider, CancellationToken, Task> has no return value, so the custom ping endpoint will return 200 OK with an empty body instead of the serialized ping response. This is a behavioral regression for users with custom [AgentCorePing] methods that return health data.
   • Suggestion: Consider changing the ping handler signature to Func<IServiceProvider, CancellationToken, Task<object>> or Func<HttpContext, Task> so the handler can write its response. Alternatively, for AOT-safe scenarios, accept a Func<IServiceProvider, CancellationToken, Task> that is expected to write directly to the HttpContext, but this needs to be documented and the HttpContext must be accessible.

2. Significant code duplication in StreamingResponseWriter

   • File: src/AWS.AgentCore/Internal/StreamingResponseWriter.cs
   • The new WriteStreamingResponseAsync(HttpContext, IAsyncEnumerable<string>) overload duplicates approximately 70 lines of SSE writing logic from the existing method. Both methods perform identical SSE header setup, chunk writing, done message, and error handling. This creates a maintenance burden where any bug fix or behavior change must be applied in two places.
   • Suggestion: Consider refactoring to extract the common SSE writing logic into a shared private method that both overloads call. The existing method could resolve the IAsyncEnumerable from the delegate and then delegate to the shared implementation.

3. No-op await used as async workaround in generated ping handler

   • File: src/AWS.AgentCore.SourceGenerator/AgentCoreStartupGenerator.cs
   • The generated code emits await System.Threading.Tasks.Task.CompletedTask; as a no-op to satisfy the async lambda requirement without compiler warnings. This is a code smell in generated output. If the ping handler doesn't need to be async, the lambda signature should be synchronous, or if async is required by the overload, the Ping method return value should be awaited.
   • Suggestion: If keeping the current approach, consider whether the source generator should check if the user's Ping method returns Task and emit an await accordingly, or emit a synchronous lambda when the method is synchronous.

Minor Issues

1. Record types use lowercase constructor parameters for JSON property names in src/AWS.AgentCore/Internal/AgentCoreJsonContext.cs
2. PromptRequest model duplicated across sample apps in sampleapps/NativeAotAnnotations/Models/PromptRequest.cs
3. JsonContextType stored at ClassInfo level rather than InvocationMethodInfo in src/AWS.AgentCore.SourceGenerator/AgentCoreStartupGenerator.cs

Positive Notes

• Excellent PR description with a comprehensive overload matrix table that clearly documents the AOT-safe vs. reflection-based options.
• Clean API design - preserving backward compatibility with reflection-based overloads while adding AOT-safe alternatives follows .NET ecosystem best practices (similar to System.Text.Json's approach).
• Good use of [RequiresUnreferencedCode] and [RequiresDynamicCode] annotations with helpful warning messages that guide users toward AOT-safe alternatives.
• The AgentCoreJsonContext with source-generated serialization for all internal response types is well-structured and eliminates all internal reflection for JSON operations.
• The source generator changes are well-implemented, with the new BuildHandlerCallArgs helper providing clean parameter mapping logic.
• Both sample apps provide clear, complete examples of AOT usage for the two programming models, including production-ready Dockerfiles using runtime-deps base image.
• Test coverage includes a new snapshot test for the WithJsonContext case, and all existing tests are maintained.

Recommendation

Request changes - please address the important issues listed above.

approved assuming double check the comment about the ping return